Skip to content

Migrate edicon to Ruby Implementation#27

Merged
Largo merged 1 commit into
masterfrom
migrate-edicon-to-ruby
Mar 18, 2026
Merged

Migrate edicon to Ruby Implementation#27
Largo merged 1 commit into
masterfrom
migrate-edicon-to-ruby

Conversation

@shinokaro
Copy link
Copy Markdown
Collaborator

This pull request introduces a Ruby-based implementation of the previously C-based edicon.exe. Opting for a Ruby reimplement over fixing the existing C code will shorten the build times and make maintenance easier, leveraging widely accessible Microsoft documentation.

Reference Information:
The original edicon.c contained several issues that are being addressed by this reimplementation:

  • BeginUpdateResource was incorrectly compared to INVALID_HANDLE_VALUE. Proper error checking requires comparison to NULL.(
    if (h == INVALID_HANDLE_VALUE)
    )
  • The calculation of GroupIconSize was incorrect. It should have been calculated using the size of IconDirResEntry multiplied by the number of images, instead of using IconDirectoryEntry.(
    DWORD GroupIconSize = sizeof(GroupIcon) + header->ImageCount * sizeof(IconDirectoryEntry);
    )

This reimplement ensures that the functionality of EdIcon is preserved while simplifying future enhancements and maintenance.

@shinokaro shinokaro force-pushed the migrate-edicon-to-ruby branch from 23b6597 to 4084207 Compare August 9, 2024 20:29
This pull request introduces a Ruby-based implementation of the previously C-based `edicon.exe`. Opting for a Ruby reimplement over fixing the existing C code will shorten the build times and make maintenance easier, leveraging widely accessible Microsoft documentation.

Reference Information:
The original `edicon.c` contained several issues that are being addressed by this reimplementation:

- `BeginUpdateResource` was incorrectly compared to `INVALID_HANDLE_VALUE`. Proper error checking requires comparison to `NULL`.
- The calculation of `GroupIconSize` was incorrect. It should have been calculated using the size of `IconDirResEntry` multiplied by the number of images, instead of using `IconDirectoryEntry`.
This reimplement ensures that the functionality of `EdIcon` is preserved while simplifying future enhancements and maintenance.
@Largo Largo force-pushed the migrate-edicon-to-ruby branch from 4084207 to 7da70cf Compare March 18, 2026 21:06
Copy link
Copy Markdown
Owner

@Largo Largo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Largo Largo merged commit 1f02036 into master Mar 18, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants